-
Notifications
You must be signed in to change notification settings - Fork 40
feat: add profile info #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Warning Rate limit exceeded@cmeesters has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 2 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds documentation describing the recommended profiles directory layout and usage: a new "The 'profiles' Directory" section in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)profiles/README.txt(1 hunks)
🧰 Additional context used
🪛 LanguageTool
profiles/README.txt
[style] ~8-~8: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ... pitfalls or other things to consider. Feel free to open pull requests for 3rd party workfl...
(FEEL_FREE_TO_STYLE_ME)
[grammar] ~9-~9: Use a hyphen to join words.
Context: ... Feel free to open pull requests for 3rd party workflows you are working with to ...
(QB_NEW_EN_HYPHEN)
[grammar] ~9-~9: Ensure spelling is correct
Context: ...n during development, e.g. plotting and dowload rules.
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
README.md
[grammar] ~59-~59: Ensure spelling is correct
Context: ...cular execution environment (cluster or cload). You may include a readme file next to...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
10-15: Add the new section to the table of contents.The "The 'profiles' Directory" section should be added to the table of contents for consistency and to help users navigate the documentation.
Apply this diff to update the table of contents:
- [Snakemake workflow: `<name>`](#snakemake-workflow-name) - [Usage](#usage) - [Deployment options](#deployment-options) + - [The 'profiles' Directory](#the-profiles-directory) - [Authors](#authors) - [References](#references) - [TODO](#todo)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)profiles/README.txt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- profiles/README.txt
🔇 Additional comments (1)
README.md (1)
53-59: Good typo fix and clear documentation.The spelling error flagged in the previous review ("cload" → "cloud") has been corrected, and the new section provides clear guidance on the profiles directory structure and purpose.
m-jahn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Christian, great idea to include profiles!
I have some general comments that could be addressed to make this even more user-friendly:
- maybe change the README.txt to a README.md in line with all other markdown docs we use for documentation? This would also include the file in automatic md linting by Github actions
- would you like to include an actual dummy profile? Or would that conflict with other parts of a workflow template? From my point of view that would be OK since profile will stay silent until it is invoked with
snakemake --profileright? If we like to encourage the use of profiles, a real working profile including a statement how to invoke it would be a really cool addition.
| - [Snakemake workflow: `<name>`](#snakemake-workflow-name) | ||
| - [Usage](#usage) | ||
| - [Deployment options](#deployment-options) | ||
| - [The "profiles" Directory](#the-profiles-directory) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
directory lower-case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - [The "profiles" Directory](#the-profiles-directory) | |
| - [Workflow profiles](#workflow-profiles) |
| snakemake --cores 2 --sdm conda apptainer --directory .test | ||
| ``` | ||
|
|
||
| ## The "profiles" Directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
directory lower-case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## The "profiles" Directory | |
| ## Workflow profiles |
|
|
||
| `profiles/<your cluster or cloud instance>/config.yaml` | ||
|
|
||
| You may include a readme file next to the config.yaml file to point out pitfalls or other things to consider. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may include a README.md file [...]
|
@m-jahn will switch to lower case and add a template for this particular workflow. Will only able to do this by the end of the week. |
|
thanks for doing this! |
dlaehnemann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think this is a great idea, and just double-checked that the profiles/ directory also gets copied by snakedeploy.
The code does it, but I had to add a PR that adds the profiles/ directory copy info to the snakedeploy docs.
And here, I just edited the text heavily and removed redundancy by making the text in the main README.md really short and pointing to the profiles/README.md for details.
Also I:
- added in a bunch of linkouts to the relevant documentation pages
- differentiate between the
default/and other profiles - differentiate between generic and cluster-specific profiles
- mention that resource specification should go directly into rules, wherever a generic (dynamic) resource specification is feasible
In general, I kept all the info that was there. The only thing I removed was this sentence:
It may also be necessary to occasionally label certain rules of a particular workflow with the
localrules: <rule 1>, <rule 2>, ...directive when workflow developers focused on server execution during development, e.g., plotting and download rules.
It feels like something that should rather go into the snakemake profiles documentation somewhere, as this is a useful strategy for using profiles during development in general, right? @cmeesters, would you like to prepare a respective docs PR on snakemake?
| - [Snakemake workflow: `<name>`](#snakemake-workflow-name) | ||
| - [Usage](#usage) | ||
| - [Deployment options](#deployment-options) | ||
| - [The "profiles" Directory](#the-profiles-directory) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - [The "profiles" Directory](#the-profiles-directory) | |
| - [Workflow profiles](#workflow-profiles) |
| snakemake --cores 2 --sdm conda apptainer --directory .test | ||
| ``` | ||
|
|
||
| ## The "profiles" Directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## The "profiles" Directory | |
| ## Workflow profiles |
| When developing on a cluster or cloud instance, please include a "profiles" directory: | ||
|
|
||
| `profiles/<cluster or cloud name>/config.yaml` | ||
|
|
||
| This configuration file should contain the workflow profile with its resource specification for a particular execution environment (cluster or cloud). You may include a readme file next to point out pitfalls or other aspects worth a user's consideration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| When developing on a cluster or cloud instance, please include a "profiles" directory: | |
| `profiles/<cluster or cloud name>/config.yaml` | |
| This configuration file should contain the workflow profile with its resource specification for a particular execution environment (cluster or cloud). You may include a readme file next to point out pitfalls or other aspects worth a user's consideration. | |
| The `profiles/` directory can contain any number of [workflow specific profiles](https://snakemake.readthedocs.io/en/stable/executing/cli.html#profiles) that users can choose from. | |
| The [profiles `README.md`](profiles/README.md) provides more details. |
| A 'profiles' directory might contain workflow resource configuration for a particular cluster or cloud instance. | ||
|
|
||
| We encourage to include a profile for your use case as: | ||
|
|
||
| `profiles/<your cluster or cloud instance>/config.yaml` | ||
|
|
||
| You may include a readme file next to the config.yaml file to point out pitfalls or other things to consider. | ||
|
|
||
| We welcome pull requests for 3rd-party workflows you are working with to include such a profile! It may also be necessary to occasionally label certain rules of a particular workflow with the `localrules: <rule 1>, <rule 2>, ...` directive when workflow developers focused on server execution during development, e.g., plotting and download rules. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| A 'profiles' directory might contain workflow resource configuration for a particular cluster or cloud instance. | |
| We encourage to include a profile for your use case as: | |
| `profiles/<your cluster or cloud instance>/config.yaml` | |
| You may include a readme file next to the config.yaml file to point out pitfalls or other things to consider. | |
| We welcome pull requests for 3rd-party workflows you are working with to include such a profile! It may also be necessary to occasionally label certain rules of a particular workflow with the `localrules: <rule 1>, <rule 2>, ...` directive when workflow developers focused on server execution during development, e.g., plotting and download rules. | |
| The `profiles/` directory can contain any number of subdirectories, each containing a `config.yaml` file with a [workflow specific profile](https://snakemake.readthedocs.io/en/stable/executing/cli.html#profiles): | |
| `profiles/<specific_profile_name>/config.yaml` | |
| The profile `profiles/default/config.yaml` will automatically be used by snakemake whenever you don't provide a workflow-specific profile via `--workflow-profile`. | |
| This means that any resources or other (command line) arguments specified there, will implicitly be used when running this workflow. | |
| Thus, as a workflow developer, only put configurations there that you expect to work in most environments, but which the users might want to tweak. | |
| And for rule-specific resource setting, preferably provide generally applicable settings right in the rule definition, if necessary via [dynamic resource](https://snakemake.readthedocs.io/en/stable/snakefiles/rules.html#dynamic-resources) specification---users can always override those in a profile, if they need to. | |
| For any more specific profiles, use separate and clearly named subdirectories. | |
| For example use `profiles/slurm/config.yaml` for a slurm-specific profile, or even something like `profiles/slurm_uni_xyz/config.yaml` for a particular institutional slurm compute cluster. | |
| It is also good practice to add clear documentation comments for each entry in a (workflow) profile. | |
| This should explain the respective entry, indicate what kind of values can be used and why a particular value or setting were chosen. | |
| To this end, it is often helpful to provide links to relevant documentation pages, either from snakemake, a snakemake plugin or a specific cluster environment. | |
| In general, we welcome pull requests for 3rd-party workflows you are working with to include such a profile for your specific compute environment. |
Snakemake workflows to be executed on cluster or cloud environments deployed with
snakedeploymeanwhile copy aprofilesdirectory, if present.This PR attempts to add an empty
profilesdirectory. It includes a brief description in theREADME.mdand adds a similar file in the profiles directory.Summary by CodeRabbit